Drop node 18 and 20. Add 24#1882
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request drops support for Node.js 18 and 20, updating the minimum supported version to 22, and upgrades various dependencies. It replaces the deprecated punycode module with node:url's domainToASCII, adopts standard node: prefix imports, and modernizes cryptography calls using crypto.hash(). Additionally, the request client is refactored to use AbortSignal.timeout() instead of manual timeouts. A potential issue was identified in src/webServiceClient.ts where the caught error in the catch block is directly cast to Error, which can cause a runtime crash if the thrown value is not an object.
mm-kevcenteno
force-pushed
the
kevin/drop-old-node
branch
11 times, most recently
from
d662584 to
52218db
Compare
mm-kevcenteno
force-pushed
the
kevin/drop-old-node
branch
from
52218db to
f11870f
Compare
| response = await fetch(url, options); | ||
| } catch (err) { | ||
| const error = err as Error; | ||
| const error = err instanceof DOMException ? err : new Error(String(err)); |
There was a problem hiding this comment.
Thoughts on this? So Error - Error: ${error can just be Error: ${Error}
| const error = err instanceof DOMException ? err : new Error(String(err)); | |
| const error = err instanceof Error || err instanceof DOMException ? err : new Error(String(err)); |
| @@ -2,7 +2,14 @@ | |||
| "name": "@maxmind/minfraud-api-node", | |||
| "version": "8.3.0", | |||
| import isEmail from 'validator/lib/isEmail'; | ||
| import isFQDN from 'validator/lib/isFQDN'; | ||
| import { ArgumentError } from '../errors'; | ||
| import { isEmail, isFQDN } from 'validator'; |
There was a problem hiding this comment.
I think we need to use validator.isEmail, etc.
https://github.com/validatorjs/validator.js#es6
import validator from 'validator';https://github.com/validatorjs/validator.js#client-side-usage
<script type="text/javascript" src="validator.min.js"></script>
<script type="text/javascript">
validator.isEmail('foo@bar.com'); //=> true
</script>Proposed diff:
--- a/src/request/email.ts
+++ b/src/request/email.ts
@@ -1,3 +1,3 @@
-import { isEmail, isFQDN } from 'validator';
+import validator from 'validator';
@@ -286,1 +286,1 @@
- if (email.address != null && !isEmail(email.address)) {
+ if (email.address != null && !validator.isEmail(email.address)) {
@@ -290,1 +290,1 @@
- if (email.domain != null && !isFQDN(email.domain)) {
+ if (email.domain != null && !validator.isFQDN(email.domain)) {| import { URL } from 'node:url'; | ||
| import isURL from 'validator/lib/isURL'; | ||
| import { ArgumentError } from '../errors'; | ||
| import { isURL } from 'validator'; |
There was a problem hiding this comment.
--- a/src/request/order.ts
+++ b/src/request/order.ts
@@ -2,1 +2,1 @@
-import { isURL } from 'validator';
+import validator from 'validator';
@@ -69,1 +69,1 @@
- if (order.referrerUri != null && !isURL(order.referrerUri.toString())) {
+ if (order.referrerUri != null && !validator.isURL(order.referrerUri.toString())) {Same as f11870f#r3357776527
| @@ -1,6 +1,6 @@ | |||
| import { URL } from 'node:url'; | |||
There was a problem hiding this comment.
| import { URL } from 'node:url'; |
| @@ -167,7 +167,6 @@ returned by the web API, we also return: | |||
| ```js | |||
| import { URL } from 'url'; // Used for `order.referrerUri | |||
There was a problem hiding this comment.
| import { URL } from 'url'; // Used for `order.referrerUri |
3 participants
package-lock.json files were rebuilt because of missing transitive dependencies that caused
npm cito fail